Skip to content

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Dec 4, 2022

Forbid RPITIT from implementations from capturing more lifetimes than the trait definitions allows.

@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2022

r? @oli-obk

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 4, 2022
@rust-log-analyzer

This comment was marked as outdated.

@cjgillot cjgillot force-pushed the rpit-mismatch branch 2 times, most recently from 56de728 to 2c08b8e Compare December 4, 2022 15:28
@rust-log-analyzer

This comment has been minimized.

@cjgillot cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2022
@cjgillot cjgillot marked this pull request as draft December 4, 2022 16:31
@cjgillot cjgillot marked this pull request as ready for review December 4, 2022 19:43
@cjgillot cjgillot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Dec 5, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 5, 2022

📌 Commit 503bc7c has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2022
Report change in RPITIT lifetime capture clauses.

Forbid RPITIT from implementations from capturing more lifetimes than the trait definitions allows.
@matthiaskrgr
Copy link
Member

I think this failed in a rollup, not certain though
#105327 (comment)

@cjgillot
Copy link
Contributor Author

cjgillot commented Dec 6, 2022

Indeed, it's buggy.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 6, 2022
@cjgillot cjgillot marked this pull request as draft December 10, 2022 07:47
@bors
Copy link
Collaborator

bors commented Dec 17, 2022

☔ The latest upstream changes (presumably #105804) made this pull request unmergeable. Please resolve the merge conflicts.

rust-cloud-vms bot pushed a commit to compiler-errors/rust that referenced this pull request Jun 30, 2023
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Jul 1, 2023
…ures, r=oli-obk

Error when RPITITs' hidden types capture more lifetimes than their trait definitions

This implements a stricter set of captures rules for RPITITs. They now may only capture:
1. Lifetimes from the impl header (both the self type and any trait substs -- we may want to restrict just to the self type's lifetimes, but the PR makes that easy to do, too)
2. Lifetimes mentioned by the `impl Trait` in the trait method's definition.

Namely, they may not mention lifetimes from the method (early or late) that are not mentioned in the `impl Trait`.

cc rust-lang#105258 which I think was trying to do this too, though I'm not super familiar with what exactly differs from that or why that one was broken.
cc rust-lang#112194 (doesn't fix this issue per se, because it's still an open question, but I think this is objectively better than the status quo, and gets us closer to resolving that issue.)

Technically is a fix for the ICE in rust-lang#108580, but it turns that issue into an error now. We can decide separately whether or not nested RPITITs should capture lifetimes from their parents.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 1, 2023
…ures, r=oli-obk

Error when RPITITs' hidden types capture more lifetimes than their trait definitions

This implements a stricter set of captures rules for RPITITs. They now may only capture:
1. Lifetimes from the impl header (both the self type and any trait substs -- we may want to restrict just to the self type's lifetimes, but the PR makes that easy to do, too)
2. Lifetimes mentioned by the `impl Trait` in the trait method's definition.

Namely, they may not mention lifetimes from the method (early or late) that are not mentioned in the `impl Trait`.

cc rust-lang#105258 which I think was trying to do this too, though I'm not super familiar with what exactly differs from that or why that one was broken.
cc rust-lang#112194 (doesn't fix this issue per se, because it's still an open question, but I think this is objectively better than the status quo, and gets us closer to resolving that issue.)

Technically is a fix for the ICE in rust-lang#108580, but it turns that issue into an error now. We can decide separately whether or not nested RPITITs should capture lifetimes from their parents.

r? ``@oli-obk``
@cjgillot cjgillot closed this Sep 23, 2023
@cjgillot cjgillot deleted the rpit-mismatch branch September 23, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants